Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add visual status indicators to the UIToggle component #2698

Conversation

Vladislavs-Silins-SAP
Copy link
Contributor

The following changes were implemented:

  1. Added SVG icons SwitchOff and SwitchOn
  2. Added visual indicators to show switch status in the UIToggle component

While we are at it, we should make toggle switches a bit bigger to better comply with "Target hit area" accessibility topic. They are too small now.

image

Add functionality to render a custom "SwitchOn" icon in the UIToggle's thumb element using ReactDOM. Updated styles and component structure to support this change and ensure compatibility.
Introduce on/off icons for the UIToggle component. Updated logic to dynamically render the correct icon based on toggle state and included new SCSS for styling.
Introduce theme-based colors for 'SwitchOn' and 'SwitchOff' icons using `COLORS.thumbOn` and `COLORS.thumbOff`. This allows the icons to adapt to the current theme, improving UI consistency and flexibility.
This removes the UIToggle.scss file and replaces its styles with inline styles in the component logic. The change simplifies style handling by consolidating them into the TypeScript file, reducing dependencies and ensuring better control over dynamic styling.
Updated UIToggle to include consistent default styles for the 'Standard' size, including dimensions and padding. Updated related tests to reflect these changes and ensure correctness. Added JSDoc comments to improve method documentation.
Updated UIToggleSizeInfo properties to be optional for flexibility. Improved documentation by adding JSDoc comments for key methods. Removed a leftover console.log statement for cleaner code.
Introduced SwitchOff and SwitchOn SVG icons. Updated the UIToggle component to include visual indicators for switch status, improving the user interface clarity.
@Vladislavs-Silins-SAP Vladislavs-Silins-SAP requested a review from a team as a code owner December 12, 2024 14:45
Copy link

changeset-bot bot commented Dec 12, 2024

🦋 Changeset detected

Latest commit: 27dd3b2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@sap-ux/ui-components Minor
@sap-ux/ui-prompting Patch
@sap-ux-private/ui-prompting-examples Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions bot and others added 3 commits December 12, 2024 14:51
Removed over 2000 lines of unused or outdated icon definitions from the Icons.tsx file to improve maintainability and reduce code bloat. These icons were no longer being used across the project, and their removal cleans up the codebase.
…buttons_no-onoff-text-is-provided' of https://github.com/SAP/open-ux-tools into feat/32111-bug-acc-2531level-aa_sap-fiori-tools_switch-buttons_no-onoff-text-is-provided
@Vladislavs-Silins-SAP Vladislavs-Silins-SAP marked this pull request as draft December 13, 2024 09:43
Refined hover and focus styles for UIToggle component, improving visual consistency by aligning with updated design tokens. Updated color variables and positioning, introduced cleaner styles for checked/unchecked states, and unified border behavior.
Refined hover styles for both checked and unchecked toggle states, ensuring proper application of colors and borders. Added fallback to `contrast` variables for better theme compatibility. Removed redundant localStorage theme settings in stories.
Adjusted UIToggle component styles to improve visual consistency, aligning with updated design tokens. Modified border widths, hover, and disabled states for both toggles and thumbs, ensuring they match the latest design guidelines.
Update padding values and improve border color logic for better appearance and consistency in the UIToggle component. Fix issues with incorrect border color assignments for both checked and unchecked states.
Updated `innerPadding` to `0 1px` and refined border color handling to include fallback `transparent`. These changes ensure consistent rendering and improve visual clarity in various states.
815are
815are previously approved these changes Dec 16, 2024
Copy link
Contributor

@815are 815are left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked:

  1. code - easy to understand. As there no any custom renderer available from flunet-ui currently do not see better way how to inject icons(alternative maybe font icons or mask-images);
  2. test is maintainer;
  3. changeset exists

small comment about string value where enum entry can be used

Replaced hardcoded icon names with UiIcons constants in UIToggle. This ensures consistency and easier maintainability for icon usage across the UI components.
815are
815are previously approved these changes Dec 16, 2024
Marked toggleRootRef as readonly to prevent unintended reassignments. This ensures immutability and improves the reliability of the component.
Copy link
Contributor

@AlinaGovoruhina AlinaGovoruhina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, well commented, unit test coverage sufficient, changeset present.

…ols_switch-buttons_no-onoff-text-is-provided
@Vladislavs-Silins-SAP Vladislavs-Silins-SAP merged commit 0f9d186 into main Dec 16, 2024
16 checks passed
@Vladislavs-Silins-SAP Vladislavs-Silins-SAP deleted the feat/32111-bug-acc-2531level-aa_sap-fiori-tools_switch-buttons_no-onoff-text-is-provided branch December 16, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants